-
Notifications
You must be signed in to change notification settings - Fork 340
Add Listen 2 support #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Listen 2 support #194
Conversation
I think it's best to wait for the full release of /cc @jonleighton |
@senny I already release |
@@ -3,4 +3,4 @@ source 'https://rubygems.org' | |||
# Specify your gem's dependencies in spring.gemspec | |||
gemspec | |||
|
|||
gem 'listen', :require => false | |||
gem 'listen', "~> 2.0.0.beta.2", :require => false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Listen 2 was released is there a need to specify the beta
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, fixed!
Good point, fixed. Please give it a try before merging it, It could have a thread issue somewhere. |
Did you see that Travis is failing? |
Yep, and it's certainly related to the thread issue. I'll have a look at it this week, I need to understand how Spring handle files modification better. |
Pre-1.0 support was broken anyway, and I don't want to maintain it.
There's still one failure with Celluloid (https://travis-ci.org/jonleighton/spring/jobs/12520312) but I'm not sure how to fix it and why it happens. @tarcieri have you already such Backtrace:
|
@thibaudgg it's possible this is an actual race in the shutdown code. cc @halorgium |
It does the same even with |
Are you using |
Mmm same issue when using There's a |
Make sure that |
Work in progress (doesn't work yet)
It seems to me that it's required well before anything: https://github.com/guard/spring/blob/ca48d23e43227b679ec8f168e1ec05f3dd39ab56/test/helper.rb#L7-L8 |
Interesting! @halorgium, any idea what might be happening here? |
I will have a look when I'm back in town. |
@halorgium Thanks in advance! |
@thibaudgg having some trouble getting the tests passing with master. In terms of this change, it seems that |
@halorgium on which system are you?
I'm not sure to understand, |
@thibaudgg I'm on Mac OS X. I would expected it to use Just like require 'listen/test'
RSpec.configure do |config|
config.before(:each) do |ex|
Listen.stop
Listen.start
end
end @tarcieri and I have not totally figured out if libraries should present |
@halorgium Yep, it should definitely work on OS X, what is your Ok I understand, I think that @tarcieri @halorgium I was asking myself the same question, now Listen only return a listener object that is not a |
@thibaudgg that sounds good to me |
I'd like to use this un-merged branch, but I don't know how to target it in my Gemfile. Thanks for help! 👍 |
@jmuheim use: gem "spring", github: "guard/spring", branch: "listen2" |
This results in:
Gemfile.lock:
|
I did an |
👍 listen 2 |
Is there any news on this and/or are there plans to continue work on this? For now I'll disable listen, but it would be great to be able to use it again. :) |
👍 will be great to see this PR merged. In order of specific of one of my project I have more then 6 envs, and even for SSD spring use a lot of resource to polling files' changes. |
👍 here too. Is there still a problem with this branch? Anything we can do to help? |
@e2 maybe you have an idea and a better understanding on that one! Would be awesome and really helpful, thanks! |
@thibaudgg - I'll see what I can do. Travis failures - couldn't reproduce locally (unit tests work without issues on my setup for rails 4.0.0) - Although I'm guessing record building has become quite fast in Listen, so the project tree might have been causing a slowdown (lots of files dues to gems in /vendor dir). Acceptance tests needed a few tweaks - basically bundler was using installed gems and not installing them into vendor/gems (and the tests were failing due to rake gem not present - just like in your case). I'm not too sure about the listen gem activation problem, but I think I can work it out. I'll get the listen2 branch working, then I'll try merging with master, then testing, then rebase and at that point I could catch up and review what really needs for listen 2x to happen in spring (I'm allergic to polling too...). |
@e2 awesome, thanks! 😍 |
All tests are green, and they should now also work locally without needing Feedback is welcome (e.g. most changes are to fix the tests or make them easier to work with - so I could create a separate PR if necessary, etc.). |
@e2 great work, thanks! @jonleighton @senny finally ready to be merged! |
@e2 will check your PR on real app ;) |
@e2 That is one awesome piece of work! Thank you for doing that! I'm looking forward to seeing it being merged! |
Hi guys, Thanks to everyone who has chipped in here, and sorry for the delay in replying. I have had in my head for a while that I'd like to extract the listen code into a separate gem. This would enable it to be maintained by separate maintainers and outside of the release cycle of spring itself. I personally do not use the listen watcher, and so I'm keen for it to be maintained by people who do. I also want to minimise the amount of code which needs to exist in the spring gem. To that end, I've extracted the existing code here: https://github.com/jonleighton/spring-watcher-listen There's a supporting spring branch here: https://github.com/rails/spring/tree/extract_listen What I suggest is the following:
How does this sound? Who will volunteer to be a maintainer? Thanks |
I personally don't use use spring, but I maintain and develop listen anyway. I'd probably drop support for Listen 1.0 though (it's so ancient, using it doesn't make sense) - so I'm 👍 for extracting spring-watcher-listen into a separate gem. So, sign me up for now. |
Yep, sounds good. I only ported the listen 1 code into the new project as that's what's in spring at the moment. But upgrading it to listen 2 and releasing that seems good to me. |
Ok, I've just released Spring 1.2.0, which includes the changes which extract Listen to the @e2 is now a collaborator on the spring-watcher-listen repo, and a gem owner on rubygems.org. He can release the Listen 2 support according to his own timetable as it's not longer dependent on the Spring project itself. Of course, if any changes need to be made in the Spring project to allow spring-watcher-listen to hook in appropriately, then I'm fully supportive of that. I'm closing this PR now. Please continue the discussion at rails/spring-watcher-listen#1. Thanks! |
Needed if user have Guard 2.0 (that depends on Listen 2.0) in there Gemfile.
@senny Wouldn't be better to keep only Listen and remove spring polling watcher completely?